Distinguish a cancelled PR-required run from a real failure - #1409

Summary

Closes ledger #095. cancel-in-progress supersedes an in-flight run on every push, and the pr-required aggregate's require_* helpers lumped the resulting cancelled in with failure, so the red was indistinguishable from a genuine break at a glance.

Measured cost, this session: four separate investigations of ::error::changes result was cancelled / ::error::static-pr result was cancelled before recognising the pattern — and PR #1401 merged straight through an unrelated red the repo had learned to ignore. That second half is the worse one: a gate that goes red for reasons unrelated to the diff spends attention every time and trains everyone to click through the one time it's real.

What the aggregate does now

Two-pass. Every requirement records into failures or cancellations before anything is reported:

• Genuine failures win, and all of them print — no longer just the first. A concurrent cancellation is demoted to a ::warning::also cancelled: … context line that cannot read as an excuse.
• Cancelled with nothing failing gets its own headline, CANCELLED with no failing job: …, which states both possible causes rather than asserting one: usually supersession (look for a newer run on the current head SHA), and if there is no newer run, it was cancelled by hand and must be re-run rather than merged past.
• Still RED in every non-success case. The set of states that pass is unchanged.

The obvious fix is unsafe, and this deliberately doesn't do it

Treating cancelled as neutral was my first instinct. It's wrong here:

• GitHub counts a SKIPPED required check as PASSING. So if: !cancelled() — the idiomatic way to make the red vanish — would make a hand-cancelled run on the current head mergeable with nothing verified.
• A cancelled job proved nothing, so green would assert verification that never happened.
• It is what #095's own Stop rule forbids: "do not relax require_success for genuine failures while doing so."

if: always() is pinned by a test with the reason recorded beside it, so the unsafe version can't be reapplied later as a cleanup.

Two defects found in review, both real, both mine

This PR's history is worth reading, because both were the same mistake — verifying the state I had in mind and not the one I hadn't:

1. The first version made ci.yml unparseable. It passed the workflow-level cancelled status function through an env: value; GitHub allows those functions only in if: conditions. The run was named .github/workflows/ci.yml instead of CI, created zero jobs, and reported a bare failure with no logs. Valid YAML and invalid Actions schema, so prettier, lint, typecheck, check:github-actions and all 432 unit files passed the broken file — hosted CI was the only thing that caught it. Reading each job's own cancelled result needs no such expression, so the fix was a deletion. A new guard now fails locally on any status-check function outside an if: across .github/workflows/**, and it immediately caught a second instance in the comment I'd written to warn about the first, because expressions are interpolated inside run: blocks too.
2. The second version hid a real failure (reported by Codex). Exiting on the first non-success meant safety cancelled + build failed announced "not a real failure" and suppressed the build break entirely — worse than the ambiguity this PR set out to remove, because the old message was merely uninformative while that one was actively wrong in the direction of ignoring a break. It also asserted supersession as fact, which is false for a hand-cancelled run. Hence the two-pass structure above: it no longer depends on my having enumerated the states correctly.

Verification

• npm run verify:cheap — Test Files 431 passed (431), Tests 4490 passed | 4 skipped (4494).
• npm run check:github-actions — GitHub Actions pin check passed.
• npm run check:ci-scope — CI change scope self-test passed.
• npm run check:ci-triage — [ci-triage] self-test passed
• npx prettier --check on all three files — All matched files use Prettier code style!
• 13 cases, mutation-proven. Four fail against the first-exit version Codex reviewed, and one fails against the invalid-expression version. Cases that pass both before and after are invariants (e.g. "no cancelled required job can ever exit 0"), which is what they're for.
• The workflow parses again, confirmed on hosted CI — the only way it could be. Change scope runs, which is the first job ci.yml schedules; the two broken runs created zero jobs.
• Empty-array safety under set -euo pipefail checked explicitly rather than assumed, since the rewrite introduced bash arrays: ${#failures[@]} on an empty array is safe on bash 5.2.21, which ubuntu-24.04 ships, and the all-green case exits 0 with both arrays empty.
• The green-baseline fixture was wrong on first run (safety is required whenever DOCS_ONLY=false, and I'd set it skipped). Caught by running it; fixed the fixture, not the assertion.
• UI verification not run: no UI, routing, styling or browser behaviour changed.
• Retrieval/answer evals not run: no retrieval, ranking, selection or answer-generation change.
• npm run check:production-readiness not run: no clinical workflow, privacy, environment, Supabase, source-governance or deployment change.

The tests execute the aggregate rather than grepping the YAML

The cases extract the aggregate's run: block and run it under bash with synthetic job results. That's deliberate: both defects were behavioural, and a structural assertion would have passed against both — the exact complaint in #094 (design-system gates assert structure, not rendered effect). Coverage: green baseline; supersession; a single cancelled job; a genuine failure keeping its plain message with no cancellation excuse; the mixed cancelled-and-failed run; every failure listed rather than the first; the invariant that no cancelled required job can exit 0; the if: always() pin; and the workflow-expression rule. One further case asserts the extraction is non-empty, so a YAML restructure can't make the group vacuously green.

Risk and rollout

• Risk: low, and confined to reporting. No pass/fail outcome changes for any job state — cancelled failed before and fails now. The realistic failure mode is the test's script extraction going stale against a YAML restructure, which is why the non-empty guard exists.
• Rollback: revert this PR. pr-required returns to describing superseded runs as plain failures, one at a time.
• Provider or production effects: None to production. This edits a CI workflow, so it proves itself on hosted CI; no provider-backed gate was run from here.

Clinical Governance Preflight

Not applicable to this diff. The changed paths are .github/workflows/ci.yml, tests/ci-cache-safety.test.ts, and docs/outstanding-issues.md. No ingestion, answer generation, search/ranking, source rendering, document access, privacy or clinical output behaviour changed — the aggregate only reads other jobs' results and reports. Note this does not weaken any required check: the set of states that pass is unchanged.

Notes

RAG impact: no retrieval behaviour change — nothing under src/lib/rag/**, clinical-search, retrieval-selection, released-search-order, ranking-config, answer-ranking, the eval harness, the golden fixture, or the retrieval RPCs is touched.

This description was rewritten at head 1d415729; the original described the first attempt's RUN_CANCELLED wiring and cancelled_error helper, neither of which still exists. A stale body on a PR whose whole subject is "make CI signals honest" was worth correcting.

Previously listed here as related-and-unfixed: the ci/circleci: verify red on docs-only PRs. Moot — CircleCI was removed from the repo in #1412.

🤖 Generated with Claude Code

https://claude.ai/code/session_012YRCXgX4AWZ579bKN6sk6b